Conversation
Small Adjustment as requested by @TheS1R
@ExtremeFiretop Can you check to make sure that this commit was merged? I don't see the new eye (or new positioning) after updating. |
Hi @TheS1R it actually isn't merged, this PR is pending Martinskis approval before being merged into the "WebFun" branch in case he finds any bugs I didn't. Once merged then you will see the new eye 👁️ Hehe 😁 |
I like @TheS1R's suggestion and the general idea is good. However, the current implementation places the "eye" image actually inside the text box which means that when entering a very long password, the last chars of the input field are obscured by the "eye" image. See the screenshot below as an example: Granted, it's unlikely that users will enter very long passwords (and, AFAIK, the current maximum for ASUS routers is 32 chars, but this could change at any time, especially with newer routers having lots of storage & RAM). In any case, the implementation has a flaw in that the "eye" image is literally inside the text box. Also a question: Why make the input text box for postponement days much wider than it needs to be? |
To match the lower text box for HTML format. It's prettier to the eyes! 🤩 Lots of eye talk today |
|
Here's a screenshot of a very good design & implementation for the password input field (from the SNB Forums login panel). Notice how the "eye" image is not literally inside the input text box. It seems to be part of the input field, but it's actually a separate element that gets "merged" with the input field. So it does not obscure the text at all. |
To be fair; this is the implementation I was copying originally: Which DOES have it on the right side :P |
So are you suggesting to widen the textbox containing the eye to accomodate 32 chars (there appears to be space within the current layout)? Every eye that I have seen is within the text box (or so it appears). Another possibility might be to have two adjacent boxes that appear to be one — the first would be the text entry box, and the second would contain only the eye. Or, maybe we're overthinking this one... |
|
I found the issue; fixed in commit: |
|
Great catch btw! See @TheS1R this is why I wait for approval ;) |
But the element for "Email Format" is on a separate panel and used for a different purpose, and it's not actually a text box but a select box. The expected input type & properties are different. I just do not see the comparison. I think we have a different "philosophy" WRT website design. I lean toward "function over form" while still considering overall functionality and user experience. I still like it to be "pretty" but not simply for the sake of it. My 2 cents. |
I liked this implementation as well. Simple, functional & unobtrusive. |
|
Heading out for 30 minutes for some stuff at shoppers. Will be back soon, you guys discuss which implementation you like best (the current fixed one) or the previous one. This current implementation fixes Martinskis bug, and addresses @TheS1R 's nitpick. As for my nit pick, it was the size of the boxes, they are on different panels but on the same page, once you see it you can't unsee it hahaha 🤣 my OCD told me they had to match even if the box was larger than needed for a 3 character number |
I have an idea when I return for my nitpick! Give me a moment to drive there and back, shouldn't be more than a few minutes to fix. All I gotta do, is shrink the HTML format box!!! Easy to do. I remove "plain text" for "text". Then both options become 4 characters long and the entire box can be shrunk down to match the top one! |
Widening the text box would be a temporary solution, more like a "bandaid" to the bad design. As mentioned, ASUS could decide tomorrow to increase the maximum to 64 chars or 100 chars. The best solution is to fix it so the "eye" is outside or not part of the input text box, to avoid interfering with the input text. |
Personally, I prefer the current implementation ("fixes Martinskis bug, and addresses @TheS1R 's nitpick"), but either is functional. |
Excellent!! This addresses the issue. The "eye" image is no longer obscuring long input text. |
My bug??? LOL!! (Yeah, I know what you meant!!!! :>). |
But the correct technical term is "plain text" as opposed to "rich text" or any other type of formatted/HTML text. That would be like insisting on making the top 2 side-by-side panels on the top be of the same width (50% each) just for the sake of it, without taking into account that they each contain a different type of configuration settings (dynamic width/length vs. fixed width/length). |
Well, that by itself is not a technical reason to make them the same width. LOL!! |
I'm back! Then this? |
The bug you reported ;) |
OCD isn't a technical reason?!? LOL |
I'm shocked! How has someone never told me this?!?! ANGRY FACE EMOJI |
This to make makes the HTML format box look over sized instead of the postpone period honestly :P |
Honestly, it does not bother me at all to see different box sizes, and seeing them both have the same size has no effect on me or my user experience. I'm more focused on functionality. |
Okay one thing at a time! :P Lets start with this PR for the eye, votes! |
|
Help a brah out! |
The original implementation was fine; so it's the now-fixed latest implementation. The goal was to avoid making the "eye" obscure any user input. I'd say keep this final implementation - this part is good to go live!!! |
Okay I'm fine with this too, honestly unless we find a bug we can just roll it back, but if this makes @TheS1R happy, and addresses your reported bug. I'm happy! Now onto the next concern, if you really don't like the box widths being matched I'll rollback the commit in this PR, but I just wanted to explain myself why I matched the boxes. It may not have a been a technical reason per say, but while I was fixing the nitpick for Tom, I figured I'd fix the one that was bothering me as well. We have 3 choices.
Final verdicts? I'll implement whatever is decided |
I prefer (2), but any of three choices work. |
Oh, brother!!! If you could be a "fly on the wall" listening in during some of our engineering meetings where we discuss at length some of the changes or additions proposed either by some customer or one of our own engineers. Sometimes, the discussions can get quite "lively" (but still professional). We always have to keep in mind that "this is not a personal attack." We just have different perspectives so the ultimate determination comes down to: This is where I'm coming from and how I view things. :>) |
I'm sure I have similar meetings sometimes with the technical 4's (gov architects) Sometimes it makes me want to rip my hair out, but I also understand and can adapt. My OCD isn't the center of the world, I get that LOL. I just wanted to see what it would look like, it calmed my OCD so I submitted it. But if I need to bend a knee here I'll survive trust me 😉 |
The original code is perfectly fine to me. My 2 cents. |
Martinski4GitHub
left a comment
There was a problem hiding this comment.
Good to go live!!
It's not about needing or having "to bend a knee." At least that's now how I look at it, at all. |
Totally understand! I meant it more in a joking fashion, that in my real life work there is many times I think something is better for the department, for the public, etc. Being a public servant I often times have emotional reactions to things we do I disagree with fundamentally. But I've just learned to "bend a knee" and understand that due to reasons outside of my control or understanding, it's just the way it will be done. Now I obviously don't mean the same thing in this example, I was just using the joke as a means to say that it's a small thing and I'm willing to roll back the change understanding that it was made without a true technical reason and more of an emotional one |
Ah, OK. I got it!! It was still good to explain how I view things and what my own take is, so we can both understand each other better. Have a good night, bud!! (I know it's already late for you). |
No worries, I understand the viewpoint and I appreciate the explanation! I always know your not trying to be hard on me or anything, more of an instructor trying to pass on the views that work in this line of work! 😉 it's the reason your the official co-author to this amazing tool we have created together. With that in mind, you did merge this PR in without me revering the changes, but I can still rollback the change at any point and like I brushed on, I won't be hurt by making that rollback one bit. (Just give me the official word) As for sleeping tonight it is getting late, I'll probably poke at this more tomorrow! Have a goodnight buddy! |
When I reviewed this PR (#387), I didn't find any change for the width of the postponement days text box, so I decided to approve & merge it as it was all good to go. It looks like you made a completely separate commit solely for that one change without making a PR (see commit [c61b790). So, to be clear, that's the only commit that would be rolled back; everything else is still good to go.
Have a good night's sleep, bud. |















Small Adjustment as requested by @TheS1R